Add description to inspections. #281
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I like it! |
… and matching inspections.
|
Is there any way to cooperate with you on this one? I find it hard to guess where you are on this one - e.g. is there some range of inspections we could take care of (by adding texts)? |
Thanks for asking, @mccartney - I think this is finished now. Here's a brief summary:
|
mwz
changed the title
|
Since #298 is now addressed, I'll add 2.13.0 back to the build matrix. |
mccartney
approved these changes
Mar 12, 2020
| text = "Missing final modifier on case class", | ||
| defaultLevel = Levels.Info, | ||
| description = "Checks for case classes without a final modifier.", | ||
| explanation = "Using case classes without final modifier can lead to surprising breakage." |
There was a problem hiding this comment.
That's very vague. I guess we can improve it later (in a separate PR)
src/main/scala/com/sksamuel/scapegoat/inspections/LonelySealedTrait.scala
Outdated
Show resolved
Hide resolved
| text = "Variable shadowing", | ||
| defaultLevel = Levels.Warning, | ||
| description = "Checks for multiple uses of the variable name in nested scopes.", | ||
| explanation = "Variable shadowing is very useful, but can easily lead to nasty bugs in your code. Shadowed variables can be potentially confusing to other maintainers when the same name is adopted to have a new meaning in a nested scope." |
There was a problem hiding this comment.
Hmm, this is surprising:
Variable shadowing is very useful
There was a problem hiding this comment.
Obviously this is quite subjective like a lot of other inspections. Some languages disallow variable shadowing, some allow it in some form. I personally find it useful in small and isolated scopes where otherwise I would have to make up an arbitrary name because another identifier in the scope is already using it.
src/main/scala/com/sksamuel/scapegoat/inspections/collections/AvoidSizeEqualsZero.scala
Outdated
Show resolved
Hide resolved
| text = "Avoid Traversable.size == 0", | ||
| defaultLevel = Levels.Warning, | ||
| description = "Checks for use of Traversable.size.", | ||
| explanation = "Traversable.size can be slow for some data structure, prefer Traversable.isEmpty, which is O(1)." |
There was a problem hiding this comment.
Suggested change
| explanation = "Traversable.size can be slow for some data structure, prefer Traversable.isEmpty, which is O(1)." | |
| explanation = "Traversable.size can be slow for some data structure, prefer .isEmpty, which is O(1)." |
src/main/scala/com/sksamuel/scapegoat/inspections/collections/AvoidSizeNotEqualsZero.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/sksamuel/scapegoat/inspections/collections/JavaConversionsUse.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/sksamuel/scapegoat/inspections/collections/NegationIsEmpty.scala
Outdated
Show resolved
Hide resolved
src/main/scala/com/sksamuel/scapegoat/inspections/collections/NegationNonEmpty.scala
Outdated
Show resolved
Hide resolved
…AvoidSizeEqualsZero.scala
…AvoidSizeNotEqualsZero.scala
…NegationNonEmpty.scala
…JavaConversionsUse.scala
…NegationIsEmpty.scala
|
Thanks, @mccartney for fixing all the typos. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@sksamuel @mccartney I've started adding descriptions to inspections, but I wanted to check with you first if you're happy with this approach before I do this for all of the inspections. I've opened this draft with just few inspections updated as an example.
Here are the key changes:
I've added a
descriptionto theInspectionclass and made theexplanationnon-optional - I think every inspection should have it. I'm also planning to remove code snippets from all the inspections and just use the description to create each warning (maybe except for some cases where the message can be more specific, e.g. theCollectionNamingConfusion).It might take a while to apply those changes across all of the inspections, so I wanted to make sure you're both happy with this approach before I invest the time into this PR.
Relates to #219.